Test tooling: Report line number when expected_results.txt contains format errors
#6002
Conversation
StephanTLavavej
added
enhancement
|
|
||
| def _parse_env_lst(env_lst: Path, ctx: _ParseCtx): | ||
| for line in parse_commented_file(env_lst): | ||
| for line_number, line in _parse_commented_file(env_lst): |
There was a problem hiding this comment.
Pylance reports that line_number is unused. Using _ avoids this.
| m = _EXPECTED_RESULT_REGEX.match(line) | ||
| if m is None: | ||
| raise Exception(f'Incorrectly formatted line "{line}" in {filename}.') | ||
| raise Exception(f'Incorrectly formatted line {line_number}: "{line}" in {filename}.') |
There was a problem hiding this comment.
I have no objection to these changes, but how are they taking effect without my recently added check_libcxx_paths.py finding problems earlier during configuration? (It implicitly expects properly-formatted lines and is strict about seeing FAIL or SKIPPED so if it sees something unexpected like WOOF or SKIP then it thinks the whole line is a filename, and reports a missing file.)
There was a problem hiding this comment.
Easy answer (I think obvious to you) - I did not really pay attention to the broader context of all involved python utilities, so I did not see this interaction.
Now, after review, I can see what you're hinting at. I think we should provide my proposed diagnostics in both cases - when running cmake and when running stl-lit. That suggests some degree of merging (at least partially) check_libcxx_paths and file_parsing.py; or at least one calling functionality from the other.
I will try to implement something, probably on Thursday. I think this has a rather low priority since it only affects us. Until then I converted this PR to draft. Feel free to close, if you prefer.
github-project-automation
bot
moved this from Initial Review
to Work In Progress
in STL Code Reviews
2 participants
When a formatting error is encountered while parsing
expected_results.txt, report the line number as well.The situation notably improved in #5948, but there is still a tiny improvement to be made.